Support IOS setup in AppDelegate.m by jonastelzio · Pull Request #394 · react-native-webrtc/react-native-callkeep

jonastelzio · 2021-04-05T13:47:28Z

On IOS, CallKit doesn't get it's delegate registered before actively calling .setup from react native, meaning that we'll never get any didLoadWithEvents fired for events such as answer and end, since CallKit simply isn't delegated to callkeep prior answering the call.

This PR exposes setup so it can be called from AppDelegate.m, allowing us to catch these events as soon as they happen.

Example of how to perform setup during AppDelegate initialization:

@implementation AppDelegate
- (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(NSDictionary *)launchOptions
{ 
  self.bridge = [[RCTBridge alloc] initWithDelegate:self launchOptions:launchOptions];

  RCTRootView *rootView = [[RCTRootView alloc] initWithBridge:self.bridge
                                                   moduleName:@"App"
                                            initialProperties:nil];

  // ======== OTHER CODE REDACTED ==========

  // RNCallKeep setup should (probably?) be called before registering for PuskKit notifications
  [RNCallKeep setup:@{
    @"appName": @"Awesome App",
    @"maximumCallGroups": @3,
    @"maximumCallsPerCallGroup": @1,
    @"supportsVideo": @NO,
  }];

  // Register for pushkit. As soon as the delegates are registered, Pushkit events will fire.
  // It probably(?) doesn't happen syncronously, but RNCallKeep should probably be registered first
  [RNVoipPushNotificationManager voipRegistration];

  // ======== OTHER CODE REDACTED ==========
  
  return YES;
}

Romick2005 · 2021-04-05T14:34:03Z

I did the same in my PR #382 (comment)

jonastelzio · 2021-04-05T15:00:36Z

I did the same in my PR #382 (comment)

That pull request is really difficult to review because you changed indentation so many places in the file ;)

Romick2005 · 2021-04-05T15:18:47Z

Yep, but that is just indentation and in should look better in a good diff viewer. I do that because I am convinced that more readable and more intuitive code would be better for everyone.

danjenkins · 2021-04-05T15:28:17Z

To be honest it doesn't matter what you think the indentation in the current project; it's good practice not to just go changing it, especially within a PR for another feature

Romick2005 · 2021-04-05T15:32:02Z

I also don't care what you are thinking. I just prefer have better code in the repo that I need to use and that's it.

danjenkins · 2021-04-05T15:33:29Z

That's a great way to talk to one of the people that would approve your PR...

…ionEventListenerAdded variable

jonastelzio · 2021-04-05T16:04:10Z

 + (BOOL)isCallActive:(NSString *)uuidString;

+ (void)setup:(NSDictionary *)options;
+


Expose a static method to call setup from AppDelegate.m

jonastelzio · 2021-04-05T16:05:26Z

    if (self = [super init]) {
        _isStartCallActionEventListenerAdded = NO;
-        _delayedEvents = [NSMutableArray array];
+        if (_delayedEvents == nil) _delayedEvents = [NSMutableArray array];


init is called repeatedly, deleting any events queued up here. This avoids that.

jonastelzio · 2021-04-05T16:06:08Z

+            name, @"name",
+            body, @"data",
+            nil
+        ];


body can be null for certain events, such as RNCallKeepDidActivateAudioSession which will fire before the bridge is up if answering a call really quickly!

jonastelzio · 2021-04-05T16:08:03Z

                         supportsDTMF:(BOOL)supportsDTMF
                     supportsGrouping:(BOOL)supportsGrouping
-                   supportsUngrouping:(BOOL)supportsUngrouping)            
+                   supportsUngrouping:(BOOL)supportsUngrouping)


Whatever was sitting at the end of this line, xcode insists on deleting it when I save the file 🤷‍♂️

whitespace is the devil

jonastelzio · 2021-04-07T13:21:39Z

Upon further investigation, it might be sufficient to just call [RNCallKeep initCallKitProvider] instead of setup, considering it's using NSUserDefaults to obtain these settings.

I'll make some testing and amend the PR if that's the case.

Edit:
Doing so would cause [self.callKeepProvider setDelegate:self queue:nil]; to be executed WHILE a call is ringing, which I doubt is how Apple intended for this library to be used(?)

danjenkins · 2021-04-09T10:51:46Z

@jonastelzio can you make changes to the README too? Other than that I've confirmed these changes work in a project so I'd be happy to approve once theres docs on using setup in app delegate

jonastelzio · 2021-04-09T10:52:12Z

@jonastelzio can you make changes to the README too? Other than that I've confirmed these changes work in a project so I'd be happy to approve once theres docs on using setup in app delegate

Can do!

… only be ignored if initial setup was done in AppDelegate.m

jonastelzio · 2021-04-09T12:23:57Z

+    RNCallKeep *callKeep = [RNCallKeep allocWithZone: nil];
+    [callKeep setup:options];
+    isSetupNatively = YES;
+}


Simply allocate the singleton and call setup in there, setting isSetupNatively makes the library ignore further calls to setup from react native.

jonastelzio · 2021-04-09T12:24:23Z

+        NSLog(@"[RNCallKeep][setup] already setup");
+        RCTLog(@"[RNCallKeep][setup] already setup in native code");
+#endif
+        return;


If we've already initialized from AppDelegate.m, we ignore further calls to setup from React Native.

jonastelzio · 2021-04-09T12:26:12Z

@danjenkins I've added documentation and changed the behavior so we ONLY prevent repeated calls to .setup from react native IF the module is initially setup from native code. This way this push will never change any existing functionality.

gobzila · 2021-04-29T10:07:25Z

Any updates on this? It would be great if this PR will be merged.

manuquentin · 2021-05-02T19:51:48Z

Thanks @jonastelzio !

Implemented support for setup in AppDelegate

19413af

Adjusted if-statement in init so it doesn't touch the _isStartCallAct…

479046c

…ionEventListenerAdded variable

jonastelzio commented Apr 5, 2021

View reviewed changes

Comment thread ios/RNCallKeep/RNCallKeep.m

jonastelzio commented Apr 5, 2021

View reviewed changes

Reverted changes to packages.json

4690abc

jonastelzio marked this pull request as ready for review April 6, 2021 08:31

jonastelzio marked this pull request as draft April 7, 2021 13:22

jonastelzio marked this pull request as ready for review April 7, 2021 15:18

Added documentation and change code so subsequent calls to setup will…

e5607f5

… only be ignored if initial setup was done in AppDelegate.m

jonastelzio force-pushed the feature/ios-setup-in-appdelegate branch from 26c6990 to e5607f5 Compare April 9, 2021 12:19

jonastelzio commented Apr 9, 2021

View reviewed changes

sboily mentioned this pull request Apr 15, 2021

Add onEventHandler block handler to handle all fired events. #382

Closed

sboily added the need review Need to review ticket label Apr 15, 2021

sboily assigned sboily and manuquentin and unassigned sboily Apr 15, 2021

sboily requested a review from manuquentin April 15, 2021 14:10

sboily unassigned manuquentin Apr 15, 2021

DanijelBojcic added a commit to DanijelBojcic/react-native-callkeep that referenced this pull request Apr 15, 2021

added PR react-native-webrtc#395 and react-native-webrtc#394

c276895

manuquentin approved these changes May 2, 2021

View reviewed changes

manuquentin merged commit 6323479 into react-native-webrtc:master May 2, 2021

MEsteves22 mentioned this pull request Sep 2, 2021

*didLoadWithEvents* is not working. #311

Open

3 tasks

		+ (BOOL)isCallActive:(NSString *)uuidString;

		+ (void)setup:(NSDictionary *)options;

Conversation

jonastelzio commented Apr 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Romick2005 commented Apr 5, 2021

Uh oh!

jonastelzio commented Apr 5, 2021

Uh oh!

Romick2005 commented Apr 5, 2021

Uh oh!

danjenkins commented Apr 5, 2021

Uh oh!

Romick2005 commented Apr 5, 2021

Uh oh!

danjenkins commented Apr 5, 2021

Uh oh!

jonastelzio Apr 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonastelzio Apr 5, 2021

Choose a reason for hiding this comment

Uh oh!

jonastelzio Apr 5, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jonastelzio Apr 5, 2021

Choose a reason for hiding this comment

Uh oh!

danjenkins Apr 9, 2021

Choose a reason for hiding this comment

Uh oh!

jonastelzio commented Apr 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danjenkins commented Apr 9, 2021

Uh oh!

jonastelzio commented Apr 9, 2021

Uh oh!

jonastelzio Apr 9, 2021

Choose a reason for hiding this comment

Uh oh!

jonastelzio Apr 9, 2021

Choose a reason for hiding this comment

Uh oh!

jonastelzio commented Apr 9, 2021

Uh oh!

gobzila commented Apr 29, 2021

Uh oh!

manuquentin commented May 2, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jonastelzio commented Apr 5, 2021 •

edited

Loading

jonastelzio Apr 5, 2021 •

edited

Loading

jonastelzio commented Apr 7, 2021 •

edited

Loading